Conversation
|
""" WalkthroughA paginated article retrieval feature was implemented, including a new GET endpoint for fetching a user's articles in descending creation order. Supporting repository, service, and DTO classes were added or extended to enable pagination and sorting. Swagger configuration was updated to include a local server, and corresponding tests were introduced. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant Usecase
participant Service
participant Repository
Client->>Controller: GET /articles?pageNumber=&pageSize=
Controller->>Usecase: getAllArticles(user, pageNumber, pageSize)
Usecase->>Service: findAll(userId, PageRequest)
Service->>Repository: findAllCustom(userId, Pageable)
Repository-->>Service: Page<Article>
Service-->>Usecase: Page<Article>
Usecase-->>Controller: ArticleAllResponse (total, list)
Controller-->>Client: ResponseDto<ArticleAllResponse>
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/main/java/com/pinback/pinback_server/domain/article/domain/service/ArticleGetService.java (1)
31-33: Consider adding parameter validation for robustness.The method implementation correctly delegates to the repository layer and maintains proper transaction semantics. However, consider adding null checks for the parameters to prevent potential
NullPointerExceptionat runtime.public Page<Article> findAll(UUID userId, PageRequest pageRequest) { + if (userId == null) { + throw new IllegalArgumentException("User ID cannot be null"); + } + if (pageRequest == null) { + throw new IllegalArgumentException("Page request cannot be null"); + } return articleRepository.findAllCustom(userId, pageRequest); }src/main/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecase.java (1)
48-54: Add parameter validation to prevent IllegalArgumentException.The implementation is clean, but
PageRequest.of()can throwIllegalArgumentExceptionfor invalid parameters. Consider adding validation or let the controller handle validation as suggested in the previous comment.If you want to add validation at the use case level, apply this diff:
public List<ArticlesResponse> getAllArticles(User user, int pageNumber, int pageSize) { + if (pageNumber < 0) { + throw new IllegalArgumentException("Page number must be non-negative"); + } + if (pageSize <= 0) { + throw new IllegalArgumentException("Page size must be positive"); + } Page<Article> articles = articleGetService.findAll(user.getId(), PageRequest.of(pageNumber, pageSize));src/main/java/com/pinback/pinback_server/domain/article/domain/repository/ArticleRepositoryCustomImpl.java (1)
27-43: Well-implemented custom repository with QueryDSL.The implementation correctly handles pagination and ordering requirements. The use of a separate count query and
PageableExecutionUtils.getPageis good for performance.Consider whether the
fetchJoin()on user is necessary since you're only filtering by user ID and the ArticlesResponse doesn't include user data:List<Article> articles = queryFactory .selectFrom(article) - .join(article.user, user).fetchJoin() + .join(article.user, user) .where(article.user.id.eq(userId))This could improve performance by avoiding unnecessary data fetching.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecase.java(3 hunks)src/main/java/com/pinback/pinback_server/domain/article/domain/repository/ArticleRepository.java(1 hunks)src/main/java/com/pinback/pinback_server/domain/article/domain/repository/ArticleRepositoryCustom.java(1 hunks)src/main/java/com/pinback/pinback_server/domain/article/domain/repository/ArticleRepositoryCustomImpl.java(1 hunks)src/main/java/com/pinback/pinback_server/domain/article/domain/service/ArticleGetService.java(2 hunks)src/main/java/com/pinback/pinback_server/domain/article/presentation/ArticleController.java(2 hunks)src/main/java/com/pinback/pinback_server/domain/article/presentation/dto/response/ArticlesResponse.java(1 hunks)src/main/java/com/pinback/pinback_server/global/config/swagger/SwaggerConfig.java(1 hunks)src/test/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecaseTest.java(3 hunks)src/test/java/com/pinback/pinback_server/domain/fixture/TestFixture.java(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: rootTiket
PR: Pinback-Team/pinback-server#18
File: src/main/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecase.java:38-41
Timestamp: 2025-07-07T15:33:02.697Z
Learning: In the Pinback server project, article detail retrieval (getArticleDetail method) does not require user authorization checks. Articles can be accessed by ID without ownership verification as per the project's design decision.
src/main/java/com/pinback/pinback_server/domain/article/domain/repository/ArticleRepository.java (1)
Learnt from: rootTiket
PR: Pinback-Team/pinback-server#18
File: src/main/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecase.java:38-41
Timestamp: 2025-07-07T15:33:02.697Z
Learning: In the Pinback server project, article detail retrieval (getArticleDetail method) does not require user authorization checks. Articles can be accessed by ID without ownership verification as per the project's design decision.
src/main/java/com/pinback/pinback_server/domain/article/domain/repository/ArticleRepositoryCustom.java (1)
Learnt from: rootTiket
PR: Pinback-Team/pinback-server#18
File: src/main/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecase.java:38-41
Timestamp: 2025-07-07T15:33:02.697Z
Learning: In the Pinback server project, article detail retrieval (getArticleDetail method) does not require user authorization checks. Articles can be accessed by ID without ownership verification as per the project's design decision.
src/main/java/com/pinback/pinback_server/domain/article/presentation/ArticleController.java (1)
Learnt from: rootTiket
PR: Pinback-Team/pinback-server#18
File: src/main/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecase.java:38-41
Timestamp: 2025-07-07T15:33:02.697Z
Learning: In the Pinback server project, article detail retrieval (getArticleDetail method) does not require user authorization checks. Articles can be accessed by ID without ownership verification as per the project's design decision.
src/main/java/com/pinback/pinback_server/domain/article/domain/service/ArticleGetService.java (1)
Learnt from: rootTiket
PR: Pinback-Team/pinback-server#18
File: src/main/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecase.java:38-41
Timestamp: 2025-07-07T15:33:02.697Z
Learning: In the Pinback server project, article detail retrieval (getArticleDetail method) does not require user authorization checks. Articles can be accessed by ID without ownership verification as per the project's design decision.
src/main/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecase.java (1)
Learnt from: rootTiket
PR: Pinback-Team/pinback-server#18
File: src/main/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecase.java:38-41
Timestamp: 2025-07-07T15:33:02.697Z
Learning: In the Pinback server project, article detail retrieval (getArticleDetail method) does not require user authorization checks. Articles can be accessed by ID without ownership verification as per the project's design decision.
src/test/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecaseTest.java (1)
Learnt from: rootTiket
PR: Pinback-Team/pinback-server#18
File: src/main/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecase.java:38-41
Timestamp: 2025-07-07T15:33:02.697Z
Learning: In the Pinback server project, article detail retrieval (getArticleDetail method) does not require user authorization checks. Articles can be accessed by ID without ownership verification as per the project's design decision.
src/main/java/com/pinback/pinback_server/domain/article/domain/repository/ArticleRepositoryCustomImpl.java (1)
Learnt from: rootTiket
PR: Pinback-Team/pinback-server#18
File: src/main/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecase.java:38-41
Timestamp: 2025-07-07T15:33:02.697Z
Learning: In the Pinback server project, article detail retrieval (getArticleDetail method) does not require user authorization checks. Articles can be accessed by ID without ownership verification as per the project's design decision.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (7)
src/main/java/com/pinback/pinback_server/domain/article/domain/repository/ArticleRepositoryCustom.java (1)
10-12: LGTM! Clean custom repository interface design.The interface follows Spring Data JPA custom repository conventions perfectly. The method signature is appropriate for paginated user-specific article retrieval.
src/main/java/com/pinback/pinback_server/global/config/swagger/SwaggerConfig.java (1)
24-27: LGTM! Good addition of local server configuration.Adding the localhost server to the Swagger configuration improves developer experience by allowing API testing against both local and deployment environments. The implementation is clean and maintains the existing deployment server configuration.
src/main/java/com/pinback/pinback_server/domain/article/domain/repository/ArticleRepository.java (1)
10-10: LGTM! Correct Spring Data JPA custom repository pattern.The repository properly extends both
JpaRepositoryandArticleRepositoryCustom, following the standard Spring Data pattern for integrating custom query methods. This maintains existing functionality while enabling the new paginated retrieval feature.src/test/java/com/pinback/pinback_server/domain/fixture/TestFixture.java (1)
33-35: LGTM! Helpful test fixture method addition.The new overloaded method provides flexibility for creating articles with specific URLs, which supports testing the new pagination functionality. The implementation is consistent with existing fixture methods, maintaining the same timestamp and memo patterns.
src/main/java/com/pinback/pinback_server/domain/article/domain/service/ArticleGetService.java (1)
3-6: LGTM! Proper imports for pagination support.The new imports are appropriate for the pagination functionality being added.
src/main/java/com/pinback/pinback_server/domain/article/presentation/dto/response/ArticlesResponse.java (1)
1-17: LGTM! Clean DTO implementation with appropriate factory method.The record structure is well-designed for the article listing use case, and the static factory method follows good practices for entity-to-DTO conversion.
src/test/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecaseTest.java (1)
97-142: Excellent test coverage for the pagination feature.The tests comprehensively verify both pagination functionality and ordering requirements. The test setup is clean with proper data isolation, and the assertions are appropriate for validating the expected behavior.
| @GetMapping | ||
| public ResponseDto<List<ArticlesResponse>> getAll(@CurrentUser User user, @RequestParam int pageNumber, | ||
| @RequestParam int pageSize) { | ||
|
|
||
| List<ArticlesResponse> response = articleManagementUsecase.getAllArticles(user, pageNumber, pageSize); | ||
| return ResponseDto.ok(response); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for pagination parameters to prevent invalid requests.
The endpoint lacks validation for pageNumber and pageSize parameters, which could lead to runtime exceptions or performance issues with invalid values.
Apply this diff to add proper validation:
@GetMapping
-public ResponseDto<List<ArticlesResponse>> getAll(@CurrentUser User user, @RequestParam int pageNumber,
- @RequestParam int pageSize) {
+public ResponseDto<List<ArticlesResponse>> getAll(@CurrentUser User user,
+ @RequestParam @Min(value = 0, message = "Page number must be non-negative") int pageNumber,
+ @RequestParam @Min(value = 1, message = "Page size must be positive") @Max(value = 100, message = "Page size must not exceed 100") int pageSize) {Don't forget to add the import for validation annotations:
+import jakarta.validation.constraints.Min;
+import jakarta.validation.constraints.Max;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @GetMapping | |
| public ResponseDto<List<ArticlesResponse>> getAll(@CurrentUser User user, @RequestParam int pageNumber, | |
| @RequestParam int pageSize) { | |
| List<ArticlesResponse> response = articleManagementUsecase.getAllArticles(user, pageNumber, pageSize); | |
| return ResponseDto.ok(response); | |
| } | |
| // add at top of file alongside other imports | |
| import jakarta.validation.constraints.Min; | |
| import jakarta.validation.constraints.Max; | |
| @GetMapping | |
| public ResponseDto<List<ArticlesResponse>> getAll(@CurrentUser User user, | |
| @RequestParam @Min(value = 0, message = "Page number must be non-negative") int pageNumber, | |
| @RequestParam @Min(value = 1, message = "Page size must be positive") | |
| @Max(value = 100, message = "Page size must not exceed 100") int pageSize) { | |
| List<ArticlesResponse> response = | |
| articleManagementUsecase.getAllArticles(user, pageNumber, pageSize); | |
| return ResponseDto.ok(response); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/pinback/pinback_server/domain/article/presentation/ArticleController.java
around lines 42 to 49, the getAll method lacks validation for the pagination
parameters pageNumber and pageSize. To fix this, add validation annotations such
as @Min(1) to both parameters to ensure they are positive integers. Also, import
the necessary validation annotations from javax.validation.constraints. This
will prevent invalid requests and potential runtime errors.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/com/pinback/pinback_server/domain/article/presentation/dto/response/ArticleAllResponse.java (2)
5-8: Consider using plural form for the count field.The field
totalArticlerepresents a count of multiple articles, sototalArticleswould be more semantically accurate and consistent with thearticlesfield.public record ArticleAllResponse( - long totalArticle, + long totalArticles, List<ArticlesResponse> articles ) {
9-14: LGTM with consideration for null safety.The static factory method follows Java conventions and provides a clean way to create instances. Consider adding null checks or validation annotations if your project requires defensive programming practices.
If you want to add null safety:
public static ArticleAllResponse of(long totalArticle, List<ArticlesResponse> articles) { + Objects.requireNonNull(articles, "articles cannot be null"); return new ArticleAllResponse( totalArticle, articles ); }Don't forget to import
java.util.Objectsif you choose this approach.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecase.java(3 hunks)src/main/java/com/pinback/pinback_server/domain/article/presentation/ArticleController.java(2 hunks)src/main/java/com/pinback/pinback_server/domain/article/presentation/dto/response/ArticleAllResponse.java(1 hunks)src/test/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecaseTest.java(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/pinback/pinback_server/domain/article/presentation/ArticleController.java
- src/main/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecase.java
- src/test/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecaseTest.java
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: rootTiket
PR: Pinback-Team/pinback-server#18
File: src/main/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecase.java:38-41
Timestamp: 2025-07-07T15:33:02.697Z
Learning: In the Pinback server project, article detail retrieval (getArticleDetail method) does not require user authorization checks. Articles can be accessed by ID without ownership verification as per the project's design decision.
🔇 Additional comments (1)
src/main/java/com/pinback/pinback_server/domain/article/presentation/dto/response/ArticleAllResponse.java (1)
1-3: LGTM!Package declaration follows standard conventions and imports are minimal and appropriate.
| public Page<Article> findAllCustom(UUID userId, Pageable pageable) { | ||
| List<Article> articles = queryFactory | ||
| .selectFrom(article) | ||
| .join(article.user, user).fetchJoin() | ||
| .where(article.user.id.eq(userId)) | ||
| .offset(pageable.getOffset()) | ||
| .limit(pageable.getPageSize()) | ||
| .orderBy(article.createdAt.desc()) | ||
| .fetch(); |
There was a problem hiding this comment.
지금 구현하신 페이지네이션 방식은 offset 기반 외에도 cursor 기반이 있으니 시간이 될 때 공부하면 좋을 것 같습니당 💪💪💪
2 participants
🚀 PR 요약
아티클 전체 조회 기능 구현했습니다.
✨ PR 상세 내용
🚨 주의 사항
스웨거에 localhost 지워져 있어서 추가했습니다
✅ 체크 리스트
close #19
Summary by CodeRabbit